-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Template verifier adoption in CommonTemplateTests #28707
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
33e7101
to
d360677
Compare
Restore action executed through the TemplateVerifier fails on Ubuntu and Darwin with
Caused by not using the dotnet.exe from artifacts folder. To support custom dotnet location a VerificationEngine rework is needed that'll be done after dotnet/templating#5482 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left few comments.
In general, imo common tests should be just rewritten as snapshot tests testing all possible combination of parameters - it will cover all current tests.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
21be6c1
to
b307d23
Compare
b307d23
to
7be7c25
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the simplification of the tests and decreased test run time!
Left couple of comments, all minor; some looks like possible issues.
c58babd
to
3ccb7ba
Compare
[InlineData("global.json file", "globaljson", null)] | ||
[InlineData("global.json file", "globaljson", new[] { "--sdk-version", "5.0.200" })] | ||
[InlineData("global.json file", "globaljson", new[] { "--sdk-version", "5.0.200", "--roll-forward", "major" })] | ||
[InlineData("global.json file", "global.json", null)] | ||
[InlineData("global.json file", "global.json", new[] { "--sdk-version", "5.0.200" })] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should remove .NET 5 here (EOL), can be done in another PR
…with features conditions
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Template verifier adoption in CommonTemplateTests
Problem
Part of dotnet/templating#3868
Coverting the CommonTemplateTests from manually running processes and checking outputs to use the verification tooling
Solution
Verification tooling importent and tests converted and adjusted.
Some repetitive tests (running identical scenarios but testing different parts of outputs) were consolidated